Skip to content

Core: invoke ready watcher even if discovery fails - #719

Open
mnnekrashevich wants to merge 1 commit into
ydb-platform:masterfrom
mnnekrashevich:fix/294-ready-watcher-on-discovery-error
Open

Core: invoke ready watcher even if discovery fails#719
mnnekrashevich wants to merge 1 commit into
ydb-platform:masterfrom
mnnekrashevich:fix/294-ready-watcher-on-discovery-error

Conversation

@mnnekrashevich

Copy link
Copy Markdown

Core: invoke ready watcher even if discovery fails

Fixes #294

Problem

When a transport is created asynchronously via GrpcTransportBuilder.buildAsync(Runnable readyWatcher), the readyWatcher callback is never invoked if the initial discovery fails. The caller waits forever and the transport is left in an unusable state without any notification.

Root cause

YdbTransportImpl.startAsync(Runnable readyWatcher) schedules a task that calls discovery.waitReady(-1) and then readyWatcher.run():

scheduler.execute(() -> {
    discovery.waitReady(-1);
    readyWatcher.run();
});

YdbDiscovery.waitReady() throws IllegalStateException("Discovery failed", ...) when discovery fails. The exception is swallowed by the executor, so readyWatcher.run() is never executed.

Fix

Wrap waitReady in try/catch/finally so that readyWatcher.run() is always executed, and log the discovery failure with its cause for diagnostics:

scheduler.execute(() -> {
    try {
        discovery.waitReady(-1);
    } catch (RuntimeException ex) {
        logger.warn("Discovery failed during async transport initialization", ex);
    } finally {
        readyWatcher.run();
    }
});

The synchronous path (build()) is not affected: there the exception is propagated to the caller, which is the correct behavior.

Tests

Added asyncBuildDiscoveryErrorTest in YdbTransportImplTest: it stubs the discovery call to fail with UNAVAILABLE, builds the transport with buildAsync, runs the scheduler tasks and asserts that the ready watcher is invoked. The test fails without this fix (ready watcher is never called) and passes with it.

  • ./mvnw -pl core test — 155 tests, all green
  • Checkstyle: 0 violations

In async transport initialization (buildAsync) the ready watcher was
never called when discovery failed: YdbDiscovery.waitReady throws
IllegalStateException and the exception was swallowed by the executor,
so the caller waited forever.

Wrap waitReady in try/catch/finally so readyWatcher.run() is always
executed, and log the discovery failure with its cause.

Fixes ydb-platform#294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In async transport initialization readyWatcher will never be called in case of discovery error

1 participant